Skip to content

kconfig: Add symbols for C standard #71024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Apr 25, 2024
Merged

kconfig: Add symbols for C standard #71024

merged 7 commits into from
Apr 25, 2024

Conversation

@pdgendt
Copy link
Contributor

@pdgendt pdgendt commented Apr 3, 2024 •
edited
Loading

Add symbols to select a C standard. Next to an option, hidden symbols are available to select the minimum.
This allows subsystems or modules to select the minimum compliant C standard supported.

@zephyrbot zephyrbot added area: Architectures area: native port Host native arch port (native_sim) labels Apr 3, 2024
@zephyrbot zephyrbot requested review from aescolar and dcpleung April 3, 2024 07:46
@aescolar aescolar requested a review from stephanosio April 3, 2024 08:02
aescolar
aescolar reviewed Apr 3, 2024
View reviewed changes
Copy link
Member

@aescolar aescolar left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this seems fine to me. My only doubt is about having a new lib/c folder just for this (which would be orphaned maintainer wise)

@pdgendt
Copy link
Contributor Author

pdgendt commented Apr 3, 2024

Overall this seems fine to me. My only doubt is about having a new lib/c folder just for this (which would be orphaned maintainer wise)

I was looking for a logical place to put it, but didn't find it obvious. If a more suitable place can be suggested, I can move it.

@aescolar
Copy link
Member

aescolar commented Apr 3, 2024

Overall this seems fine to me. My only doubt is about having a new lib/c folder just for this (which would be orphaned maintainer wise)

I was looking for a logical place to put it, but didn't find it obvious. If a more suitable place can be suggested, I can move it.

What about Kconfig.zephyr ? Maybe inside the compiler options menu?

@pdgendt pdgendt mentioned this pull request Apr 3, 2024
Merged
tejlmand
tejlmand reviewed Apr 3, 2024
View reviewed changes
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for this proposal.

Two minor improvement proposals.

@pdgendt pdgendt force-pushed the compiler-cstd branch 2 times, most recently from f99592b to a43a053 Compare April 3, 2024 09:02
@zephyrbot zephyrbot added the area: Toolchains Toolchains label Apr 3, 2024
aescolar
aescolar previously approved these changes Apr 3, 2024
View reviewed changes
@zephyrbot
Copy link

zephyrbot commented Apr 3, 2024 •
edited
Loading

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@zephyrbot zephyrbot added manifest manifest-zcbor DNM This PR should not be merged (Do Not Merge) labels Apr 3, 2024
@pdgendt pdgendt force-pushed the compiler-cstd branch 2 times, most recently from 9be6acc to b6af3b4 Compare April 15, 2024 19:14
@zephyrbot zephyrbot requested a review from yonsch April 15, 2024 19:14
tejlmand
tejlmand reviewed Apr 22, 2024
View reviewed changes
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

But fail to understand the reason for the ZEPHYR_TOOLCHAIN_SUPPORTS_GNU_EXTENSIONS setting.

tejlmand
tejlmand previously approved these changes Apr 23, 2024
View reviewed changes
@pdgendt
cmake: Replace assert with fatal error message
91c46ec 
Use vanilla cmake FATAL_ERROR messages instead of assert.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt
Copy link
Contributor Author

pdgendt commented Apr 24, 2024

Rebased to resolve conflict in release notes, PTAL @tejlmand @stephanosio

stephanosio
stephanosio reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objection to this as long as the plan is to use Kconfig for handling toolchain capabilities and configurations.

Just a minor nit about the Kconfig naming.

pdgendt added 6 commits April 24, 2024 15:26
@pdgendt
kconfig: Add symbols for C standard
aae7b35 
Add symbols to select a C standard. Next to an option, hidden symbols
are available to select the minimum.
This allows subsystems or modules to select the least compliant C
standard supported.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt
kconfig: Add symbols for GNU C Extensions
94eb7d6 
Add a symbol to enable GNU C Extensions. And a hidden option for
toolchains to signal GNU Extensions support.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt
arch: posix: Select at least C11 standard
23846b0 
Replace the global CSTD property with the CSTD kconfig option to select
at least C11 standard.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt
arcmwdt: Enable GNU C Extensions
37bb792 
Replace the global CSTD property with a Kconfig symbol selection.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt
doc: releases: release-notes: 3.7: mention deprecation of CSTD
c75578f 
Add an entry that the CSTD cmake property is deprecated and replaced
with a kconfig choice.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt
tests: bindesc: Replace CSTD definition with Kconfig symbols
0a9ceb6 
Replace the bindesc testcase CSTD definitions with the newly introduced
kconfig symbols.

Signed-off-by: Pieter De Gendt <[email protected]>
@pdgendt pdgendt requested a review from tejlmand April 24, 2024 14:06
@fabiobaltieri fabiobaltieri merged commit 4d1ae8c into zephyrproject-rtos:main Apr 25, 2024
@pdgendt pdgendt deleted the compiler-cstd branch April 25, 2024 09:55
@pdgendt pdgendt mentioned this pull request Apr 25, 2024
Merged
coreboot-bot pushed a commit to coreboot/chrome-ec that referenced this pull request Apr 30, 2024
@duda-patryk
fpmcu: Change method of selecting C standard
a87867f 
Zephyr PR #71024[1] deprecated CSTD global variable for selecting C
standard. Appropriate Kconfig options should be used instead.

[1] zephyrproject-rtos/zephyr#71024

BUG=b:321092852
TEST=zmake build bloonchipper

Change-Id: I3debb2d9ffc7295ddfbbbd7a84dda51db3fd90a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/5498197
Tested-by: Patryk Duda <[email protected]>
Commit-Queue: Patryk Duda <[email protected]>
Code-Coverage: Zoss <[email protected]>
Reviewed-by: Tom Hughes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhedberg jhedberg jhedberg approved these changes

@tejlmand tejlmand tejlmand approved these changes

@stephanosio stephanosio stephanosio approved these changes

@jeremybettis jeremybettis Awaiting requested review from jeremybettis

@nashif nashif Awaiting requested review from nashif

@nordicjm nordicjm Awaiting requested review from nordicjm

@dcpleung dcpleung Awaiting requested review from dcpleung

@abrodkin abrodkin Awaiting requested review from abrodkin

@evgeniy-paltsev evgeniy-paltsev Awaiting requested review from evgeniy-paltsev

@aescolar aescolar Awaiting requested review from aescolar

@kartben kartben Awaiting requested review from kartben

@yonsch yonsch Awaiting requested review from yonsch

@57300 57300 Awaiting requested review from 57300

Assignees

@tejlmand tejlmand

Labels

area: Architectures area: Binary Descriptors area: Build System area: C Library C Standard Library area: Kconfig area: native port Host native arch port (native_sim) area: Toolchains Toolchains Release Notes To be mentioned in the release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@pdgendt @aescolar @zephyrbot @dkalowsk @tejlmand @jhedberg @stephanosio @57300 @fabiobaltieri